Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EAM API: remove appid from appinstance path #302

Merged
merged 2 commits into from
Oct 30, 2024
Merged

Conversation

gainsley
Copy link
Collaborator

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

Removes appId from the appinstances path. For retrieval, it becomes an optional parameter, allowing a single API call to get appinstances from different Apps. For delete, it is not necessary because appInstanceId is sufficient to identify the instance.

Which issue(s) this PR fixes:

Fixes #299

Special notes for reviewers:

Changelog input

 release-note

Additional documentation

docs

@gainsley gainsley changed the title remove appid from appinstance path EAM API: remove appid from appinstance path Sep 18, 2024
Copy link

github-actions bot commented Sep 18, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.03s
✅ JSON eslint-plugin-jsonc 1 0 0 1.39s
✅ JSON jsonlint 1 0 0.18s
✅ JSON prettier 1 1 0 0.9s
✅ JSON v8r 1 0 1.72s
✅ OPENAPI spectral 3 0 5.95s
✅ REPOSITORY git_diff yes no 0.54s
✅ REPOSITORY secretlint yes no 4.62s
✅ YAML yamllint 3 0 0.8s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@JoseMConde
Copy link
Collaborator

Hi @gainsley , one thing about this:
for the get and the delete method, looks good to me, but I have doubts about the post method, because if we make appId as optional how do we know which App is going to be instantiate??
If we don't add the appId on the requested body, could we instantiate all the Apps by mistake,??

@gainsley
Copy link
Collaborator Author

gainsley commented Oct 7, 2024

Hi @JoseMConde thank you for catching that, indeed my change made the appId and appZone optional which is wrong. I've added the required spec so that they are required. Thanks for reviewing!

@JoseMConde JoseMConde merged commit 86342c4 into main Oct 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EAM: remove appId from get app instances API
2 participants